Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Logs Bridge API prototype #4725

Closed
wants to merge 107 commits into from
Closed

Logs Bridge API prototype #4725

wants to merge 107 commits into from

Conversation

pellared
Copy link
Member

@pellared pellared commented Nov 17, 2023 •
edited
Loading

Towards #4696

The design doc PR: #4809

The PR contains a prototype implementation (proposed API and prototypes of its implementation and log bridges for slog, logr) with benchmarks.

Current benchmark results:

$ go test -run=^$ -bench=.
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/log/internal
cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz
BenchmarkEmit/noop/no_attrs-16          481625260                2.357 ns/op           0 B/op          0 allocs/op
BenchmarkEmit/noop/3_attrs-16           172127840               11.77 ns/op            0 B/op          0 allocs/op
BenchmarkEmit/noop/5_attrs-16           66471745                17.22 ns/op            0 B/op          0 allocs/op
BenchmarkEmit/noop/10_attrs-16           6897639               157.6 ns/op           208 B/op          1 allocs/op
BenchmarkEmit/noop/40_attrs-16           1613598               863.9 ns/op          1408 B/op          1 allocs/op
BenchmarkEmit/writer/no_attrs-16        19277108                65.10 ns/op           16 B/op          1 allocs/op
BenchmarkEmit/writer/3_attrs-16          6325507               197.1 ns/op            48 B/op          4 allocs/op
BenchmarkEmit/writer/5_attrs-16          5215214               213.0 ns/op            48 B/op          4 allocs/op
BenchmarkEmit/writer/10_attrs-16         2246091               496.2 ns/op           296 B/op          8 allocs/op
BenchmarkEmit/writer/40_attrs-16          539629              2130 ns/op            1736 B/op         26 allocs/op
BenchmarkSlog/no_attrs-16                9658542               132.0 ns/op             0 B/op          0 allocs/op
BenchmarkSlog/3_attrs-16                 5917756               196.9 ns/op             0 B/op          0 allocs/op
BenchmarkSlog/5_attrs-16                 5342199               218.0 ns/op             0 B/op          0 allocs/op
BenchmarkSlog/10_attrs-16                1227394               884.2 ns/op           816 B/op          5 allocs/op
BenchmarkSlog/40_attrs-16                 212421              4966 ns/op            6624 B/op          8 allocs/op
BenchmarkLogr/no_attrs-16               197448330                6.707 ns/op           0 B/op          0 allocs/op
BenchmarkLogr/3_attrs-16                 7464216               142.7 ns/op           128 B/op          4 allocs/op
BenchmarkLogr/5_attrs-16                 5563734               211.2 ns/op           208 B/op          5 allocs/op
BenchmarkLogr/10_attrs-16                1378154               841.4 ns/op          1024 B/op         13 allocs/op
BenchmarkLogr/40_attrs-16                 230208              4922 ns/op            6880 B/op         40 allocs/op
PASS
ok      go.opentelemetry.io/otel/log/internal   30.867s

@pellared pellared mentioned this pull request Nov 17, 2023
Closed
@pellared pellared added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Nov 17, 2023
@pellared pellared changed the title [WIP] [Proposal] Logs Bridge API design [Proposal] Logs Bridge API design Nov 17, 2023
@pellared pellared force-pushed the logs-design branch 5 times, most recently from cb8e072 to eb1e297 Compare November 17, 2023 12:29
@pellared pellared changed the title [Proposal] Logs Bridge API design [Proposal] Logs Bridge API Nov 17, 2023
@pellared
Copy link
Member Author

@open-telemetry/go-instrumentation-approvers The docs/design/log-api.md is ready for initial review.

@pellared pellared force-pushed the logs-design branch 2 times, most recently from 92b1c5e to a63a65b Compare November 17, 2023 12:56
dashpole
dashpole reviewed Nov 17, 2023
View reviewed changes
docs/design/log-api.md Outdated Show resolved Hide resolved
dashpole
dashpole reviewed Nov 17, 2023
View reviewed changes
docs/design/log-api.md Outdated Show resolved Hide resolved
@dashpole

This comment was marked as resolved.

Sign in to view
tigrannajaryan
tigrannajaryan reviewed Nov 17, 2023
View reviewed changes
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @pellared !

docs/design/log-api.md Outdated Show resolved Hide resolved
@pellared

This comment was marked as resolved.

Sign in to view
@dashpole
Copy link
Contributor

Do you agree?

Yes. Just suggesting we document it, since we often decide to revisit it

@pellared pellared changed the title [Proposal] Logs Bridge API [Proposal] Logs Bridge API design Nov 20, 2023
@pellared pellared mentioned this pull request Jan 26, 2024
Closed
@pellared pellared changed the title [DoNotMerge] Logs Bridge API prototype Logs Bridge API prototype Jan 27, 2024
@MrAlias MrAlias added the area:logs Part of OpenTelemetry logs label Feb 12, 2024
@pellared pellared added the pkg:API Related to an API package label Feb 21, 2024
@pellared
Copy link
Member Author

Logs API is implemented. Closing. The prototype may still be helpful in future (e.g. when implementing bridges).

@pellared pellared closed this Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikeblum mikeblum mikeblum left review comments

@tigrannajaryan tigrannajaryan tigrannajaryan left review comments

@remychantenay remychantenay remychantenay left review comments

@MrAlias MrAlias MrAlias left review comments

@thomasmatecki thomasmatecki thomasmatecki approved these changes

@Aneurysm9 Aneurysm9 Awaiting requested review from Aneurysm9

@evantorrie evantorrie Awaiting requested review from evantorrie

@XSAM XSAM Awaiting requested review from XSAM XSAM is a code owner

@MadVikingGod MadVikingGod Awaiting requested review from MadVikingGod

@dmathieu dmathieu Awaiting requested review from dmathieu dmathieu is a code owner

@hanyuancheung hanyuancheung Awaiting requested review from hanyuancheung

@dashpole dashpole Awaiting requested review from dashpole dashpole is a code owner

Assignees
No one assigned
Labels
area:logs Part of OpenTelemetry logs pkg:API Related to an API package Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
Go: Logs (GA)
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@pellared @dashpole @cloorc @mikeblum @tigrannajaryan @remychantenay @MrAlias @thomasmatecki @hanyuancheung